Skip to content

fix(token): deduplicate composite-owner inputs whose token IDs are filtered out - #2241

Open
EvanYan1024 wants to merge 1 commit into
LFDT-Panurus:mainfrom
Built-by-Sign:fix/input-index-dedup
Open

fix(token): deduplicate composite-owner inputs whose token IDs are filtered out#2241
EvanYan1024 wants to merge 1 commit into
LFDT-Panurus:mainfrom
Built-by-Sign:fix/input-index-dedup

Conversation

@EvanYan1024

Copy link
Copy Markdown
Contributor

Problem

InputStream.UniquePerInput() (#2148) deduplicates the member rows of a composite-owner input by (token ID, enrollment ID) and keeps every input whose token ID is nil. But Metadata.filterTransfer clones only Senders and never TokenID, and ttx marshals that eID-filtered copy for each counterparty — so on those consumers every Input carries Id == nil, the deduplication no-ops, and a composite owner's spend is summed once per member again: 40 spent from a two-member wallet sharing an enrollment ID lands as -80 in that party's movements.

Fixes #2240

Fix

The filtering is position-preserving — filterTransfer emits one (possibly empty) entry per input and Request.extractTransferInputs walks them by index — so the input's position survives filtering and no driver/metadata wire change is needed.

  • token/stream.goInput gains Index, its position within the action, mirroring the role of Output.Index. UniquePerInput keys on (token ID, enrollment ID) when the token ID is present — unchanged behavior for existing callers, including those that construct Input directly and never set Index — and falls back to (action index, input index, enrollment ID) when it has been filtered out. The two key spaces cannot mix: filterIssue keeps Inputs verbatim (token IDs included), so only transfer inputs ever lose their token ID, and the per-type action index is unique among transfers.
  • token/request.goextractTransferInputs / extractIssueInputs record the position.
  • token/stream_test.go — contract tests for both key spaces: member rows with a filtered-out token ID now collapse, distinct token IDs at equal positions stay distinct, and a row with a token ID never collides with a positionally identical row without one.
  • docs/services/storage/ttxdb.md — documents the fallback.

Testing

go build, go vet, go test -race ./token/..., and golangci-lint v2.12.2 with the repo config, all clean locally.

@AkramBitar

AkramBitar commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

Hello @EvanYan1024,

Thanks a lot for submitting the PR.

The fix looks correct to me. When a wallet is shared by two members, spending a token produces one row per member, which is then deduplicated so the token is counted only once. The problem was that the auditor receives metadata with token IDs stripped for privacy, so every row had Id == nil and deduplication failed, causing a 40-token spend to be counted as -80.

The fix adds a position (action index + input index) to each input and uses it as the fallback identifier when the token ID is missing. Since both member rows for the same token share the same position, they are correctly collapsed to -40. Other cases remain unaffected.

One minor concern: Index defaults to 0, so a future caller that forgets to set it could cause silent incorrect deduplication. I am wondering, would it be worth adding a constructor that requires Index? What do you think on this?

Regards,
Akram

@EvanYan1024

Copy link
Copy Markdown
Contributor Author

@AkramBitar Good catch. A constructor alone would not enforce this while Input remains an exported struct — callers can still build literals and bypass it. What I would suggest is the constructor plus an internal marker:

  • NewInput(actionIndex, index, id) becomes the canonical way to build an input, and also stamps an unexported "position known" flag; the extraction paths go through it.
  • UniquePerInput applies positional deduplication only to inputs carrying the flag, so a manually constructed or zero-valued input with no token ID keeps the previous keep-all behavior instead of silently collapsing.
  • A regression test locks in that hand-built zero-position inputs do not collapse into each other.

If that works for you, I'll push it.

@AkramBitar

Copy link
Copy Markdown
Contributor

@AkramBitar Good catch. A constructor alone would not enforce this while Input remains an exported struct — callers can still build literals and bypass it. What I would suggest is the constructor plus an internal marker:

  • NewInput(actionIndex, index, id) becomes the canonical way to build an input, and also stamps an unexported "position known" flag; the extraction paths go through it.
  • UniquePerInput applies positional deduplication only to inputs carrying the flag, so a manually constructed or zero-valued input with no token ID keeps the previous keep-all behavior instead of silently collapsing.
  • A regression test locks in that hand-built zero-position inputs do not collapse into each other.

If that works for you, I'll push it.

@EvanYan1024

Thanks a lot. Works for me push it.

Regards,
Akram

@AkramBitar
AkramBitar force-pushed the fix/input-index-dedup branch from 7da7e37 to 27941e9 Compare August 18, 2026 14:17
@EvanYan1024

Copy link
Copy Markdown
Contributor Author

@AkramBitar Pushed in ba27c1c3NewInput + the internal marker, with the regression test for literal-built inputs.

@EvanYan1024
EvanYan1024 force-pushed the fix/input-index-dedup branch from 8fad044 to 9bbcbb6 Compare August 18, 2026 15:08
@AkramBitar
AkramBitar force-pushed the fix/input-index-dedup branch from 9bbcbb6 to d7e450d Compare August 18, 2026 16:03
@AkramBitar

Copy link
Copy Markdown
Contributor

@EvanYan1024,

Thanks a lot for working on this PR.

Ready to merge it could you please rebase and squash the commits to one?

Regards,
Akram

…ltered out

UniquePerInput deduplicates by (token ID, enrollment ID) and keeps every
input with a nil token ID. Metadata.filterTransfer strips TokenID from the
input metadata it clones, so consumers of eID-filtered metadata get nil
token IDs and composite-owner member rows are summed once per member again.

Give Input an Index, its position within the action, and fall back to
(action index, input index, enrollment ID) as the deduplication key when
the token ID is absent. Positions survive filtering because filterTransfer
emits one entry per input. Inputs carrying a token ID keep the previous
key, so existing callers are unaffected.

Index defaults to 0, so two literal-built inputs with no token ID would
silently collapse on a position nobody set. NewInput becomes the
canonical way to build an input and stamps an unexported position-known
marker; UniquePerInput collapses on position only for marked inputs,
while literal-built inputs keep the previous keep-all behavior, locked
by a regression test.

Signed-off-by: Evan <evanyan@sign.global>
@EvanYan1024
EvanYan1024 force-pushed the fix/input-index-dedup branch from d7e450d to 4f88680 Compare August 19, 2026 01:31
@EvanYan1024

Copy link
Copy Markdown
Contributor Author

@AkramBitar Done — rebased onto main and squashed into a single commit (4f886802). Ready to merge. Thanks a lot for the review!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Composite-owner inputs still double-count when token IDs are filtered out of the metadata

2 participants